Skip to content

feat: add input validation and type hints to BaseTool name#5063

Open
Pranav334 wants to merge 1 commit intogoogle:mainfrom
Pranav334:Pranav334-patch-1
Open

feat: add input validation and type hints to BaseTool name#5063
Pranav334 wants to merge 1 commit intogoogle:mainfrom
Pranav334:Pranav334-patch-1

Conversation

@Pranav334
Copy link
Copy Markdown

This change improves the robustness of the BaseTool class by ensuring that tool names are compatible with LLM function-calling requirements.

Key Changes:

  • Added explicit str type hints to name and description in the BaseTool.__init__ method.
  • Implemented a validation check that raises a ValueError if a tool name contains spaces.

Reasoning:
Most LLMs (including Gemini) expect function names to follow standard identifier rules (no spaces). Providing a tool with a space in the name can cause silent failures or unexpected behavior during the "Reason-Act" loop. This fix catches the error at instantiation time, providing a clear guidance to the developer to use underscores instead.

Test Coverage:

  • Verified that BaseTool(name="valid_name", ...) passes.
  • Verified that BaseTool(name="invalid name", ...) raises ValueError.

This change improves the robustness of the BaseTool class by ensuring that tool names are compatible with LLM function-calling requirements.

Key Changes:
- Added explicit `str` type hints to `name` and `description` in the `BaseTool.__init__` method.
- Implemented a validation check that raises a `ValueError` if a tool name contains spaces.

Reasoning:
Most LLMs (including Gemini) expect function names to follow standard identifier rules (no spaces). Providing a tool with a space in the name can cause silent failures or unexpected behavior during the "Reason-Act" loop. This fix catches the error at instantiation time, providing a clear guidance to the developer to use underscores instead.

Test Coverage:
- Verified that `BaseTool(name="valid_name", ...)` passes.
- Verified that `BaseTool(name="invalid name", ...)` raises ValueError.
@rohityan rohityan self-assigned this Mar 30, 2026
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Mar 30, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Mar 30, 2026

Response from ADK Triaging Agent

Hello @Pranav334, thank you for your contribution!

To help us with the review process, could you please associate an issue with this PR? If one doesn't exist, please create a new issue that this PR will address.

Also, it looks like the pyink-check is failing. Please run the autoformatter script (./autoformat.sh) to fix the formatting issues.

You can find more details in our contribution guidelines.

Thanks!

is_long_running: bool = False,
custom_metadata: Optional[dict[str, Any]] = None,
):
# -CONTRIBUTION START ---
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls remove these comments

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

custom_metadata: Optional[dict[str, Any]] = None,
):
# -CONTRIBUTION START ---
if " " in name:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a regex to include all prohibited naming conventions.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok Rohit I will look into it Thanks

@rohityan rohityan added the request clarification [Status] The maintainer need clarification or more information from the author label Mar 30, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

Hi @Pranav334 , Thank you for your contribution! We appreciate you taking the time to submit this pull request.
Pls address the review comments and fix the failing workflow tests later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation request clarification [Status] The maintainer need clarification or more information from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants